test/infra/control/run-tests-isolated.bash (1)

274-286: Minor: Progress message goes only to log file.

Line 275 redirects the "Preparing coverage data directory..." message to the log file, so users won't see it during execution. This aligns with the PR objective to reduce stdout noise. Consider whether the "Running fastcov on /gcov..." message on line 282 should also go to the log for consistency, or keep it on stdout as a key progress indicator.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/run-tests-isolated.bash` around lines 274 - 286, The
"Preparing coverage data directory..." message is currently redirected to the
coverage_log while the "Running fastcov on /gcov..." message goes to stdout;
make them consistent by redirecting the latter to the same log: update the echo
">>> Running fastcov on /gcov..." to append to "${coverage_log}" (>>
"${coverage_log}" 2>&1) so progress messages for coverage steps use the same
coverage_log variable alongside the fastcov invocation that already logs to
"${coverage_log}"; locate the echo near the fastcov call (references:
coverage_log, coverage_file, nproc_val, WORKSPACE, /gcov, fastcov) and apply the
redirection.

test/infra/control/check_all_nodes.bash (2)

16-19: Quote array expansions to avoid word splitting.

While the current table names don't contain special characters, quoting array expansions prevents future issues if table names with spaces or glob characters are added.

Proposed fix

 ALL_TABLES=()
-for i in ${!TABLES[@]}; do
-    ALL_TABLES+=(${TABLES[$i]})
-    ALL_TABLES+=("runtime_"${TABLES[$i]})
+for i in "${!TABLES[@]}"; do
+    ALL_TABLES+=("${TABLES[$i]}")
+    ALL_TABLES+=("runtime_${TABLES[$i]}")
 done

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/check_all_nodes.bash` around lines 16 - 19, The loop that
appends entries to ALL_TABLES should quote array expansions to prevent
word-splitting and globbing: update the for-loop body that uses TABLES and
ALL_TABLES so it uses "${TABLES[$i]}", "${ALL_TABLES[@]}" where appropriate and
add quotes around the "runtime_${TABLES[$i]}" expansion; specifically modify the
block that iterates over TABLES (the for i in ${!TABLES[@]} loop) to append
quoted values to ALL_TABLES (use ALL_TABLES+=("${TABLES[$i]}") and
ALL_TABLES+=("runtime_${TABLES[$i]}")) so future table names with spaces or
special chars are handled safely.

---

24-28: Quote the inner loop array expansion.

Similar to the previous loop, quote ${!ALL_TABLES[@]} for robustness.

Proposed fix

 for port in ${PORTS}; do
-    for i in ${!ALL_TABLES[@]}; do
+    for i in "${!ALL_TABLES[@]}"; do
         echo "SELECT COUNT(*) FROM ${ALL_TABLES[$i]};"
     done | mysql -u admin -padmin -h 127.0.0.1 -P ${port} > /dev/null 2>&1 &
 done

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/check_all_nodes.bash` around lines 24 - 28, The inner loop
currently iterates over ${!ALL_TABLES[@]} unquoted; update the loop header to
quote the array expansion (for i in "${!ALL_TABLES[@]}"; do) so it handles
entries with spaces or special chars safely—modify the loop that references
ALL_TABLES in the section that iterates ports (uses PORTS) to use the quoted
form.

test/infra/control/start-proxysql-isolated.bash (3)

132-142: Use [[ instead of [ for conditional tests in the node wait loop.

Same issue as the primary wait loop - use [[ for consistency and safety.

Proposed fix

-    while [ $COUNT -lt $MAX_WAIT ]; do
+    while [[ $COUNT -lt $MAX_WAIT ]]; do
         if docker exec "${PROXY_CONTAINER}" mysql -uadmin -padmin -h127.0.0.1 -P${ADMIN_PORT} -e 'SELECT 1' >/dev/null 2>&1; then
             echo " OK."
             break
         fi
         echo -n "."
         sleep 1
         COUNT=$((COUNT+1))
     done
-    if [ $COUNT -ge $MAX_WAIT ]; then echo " TIMEOUT (node ${i})"; exit 1; fi
+    if [[ $COUNT -ge $MAX_WAIT ]]; then echo " TIMEOUT (node ${i})"; exit 1; fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 132 - 142, The
node wait loop uses the single-bracket test; update the conditional expressions
to use double-bracket tests for safety and consistency: replace the while test
that checks COUNT vs MAX_WAIT (while [ $COUNT -lt $MAX_WAIT ]) with while [[
$COUNT -lt $MAX_WAIT ]] and replace the final timeout if test (if [ $COUNT -ge
$MAX_WAIT ]) with if [[ $COUNT -ge $MAX_WAIT ]]; keep the rest of the loop
(docker exec against PROXY_CONTAINER using ADMIN_PORT, the COUNT increment and
break) unchanged so behavior of the node wait loop remains the same.

---

112-125: Use [[ instead of [ for conditional tests.

Per SonarCloud analysis, [[ is safer and more feature-rich in bash scripts (handles word splitting and globbing automatically).

Proposed fix

-while [ $COUNT -lt $MAX_WAIT ]; do
+while [[ $COUNT -lt $MAX_WAIT ]]; do
     if docker exec "${PROXY_CONTAINER}" mysql -uadmin -padmin -h127.0.0.1 -P6032 -e 'SELECT 1' >/dev/null 2>&1; then
         docker exec "${PROXY_CONTAINER}" mysql -uadmin -padmin -h127.0.0.1 -P6032 -e "
             SET clickhouse-mysql_ifaces='0.0.0.0:8000';
             LOAD CLICKHOUSE VARIABLES TO RUNTIME;
         " >/dev/null 2>&1 || true
         echo " Ready."
         break
     fi
     echo -n "."
     sleep 1
     COUNT=$((COUNT+1))
 done
-if [ $COUNT -ge $MAX_WAIT ]; then echo " TIMEOUT"; exit 1; fi
+if [[ $COUNT -ge $MAX_WAIT ]]; then echo " TIMEOUT"; exit 1; fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 112 - 125,
Replace the POSIX [ ] tests with bash [[ ]] tests to avoid word-splitting and
globbing issues: update the while condition (while [ $COUNT -lt $MAX_WAIT ]; do
→ while [[ $COUNT -lt $MAX_WAIT ]]; do), the inner if that checks the docker
exec exit code (if docker exec ...; then remains but any other [ ] tests should
use [[ ]]), and the final timeout check (if [ $COUNT -ge $MAX_WAIT ]; then → if
[[ $COUNT -ge $MAX_WAIT ]]; then). Ensure all corresponding closing brackets are
switched and that variables like COUNT, MAX_WAIT and PROXY_CONTAINER are still
referenced the same way.

---

144-145: Use [[ for the numeric comparison.

Proposed fix

-if [ "${NUM_NODES}" -gt 0 ]; then
+if [[ "${NUM_NODES}" -gt 0 ]]; then

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 144 - 145,
Replace the single-bracket test in the conditional that checks NUM_NODES (the
line containing [ "${NUM_NODES}" -gt 0 ]) with a bash double-bracket numeric
comparison; i.e., use [[ ... ]] with the same operands so the conditional
correctly uses bash's arithmetic test syntax in start-proxysql-isolated.bash.

test/tap/groups/legacy-clickhouse/env.sh (1)

1-3: Add a shebang for shell identification.

Although this file is sourced rather than executed directly, adding a shebang helps static analysis tools and clarifies the intended shell dialect.

Proposed fix

+#!/bin/bash
 # Legacy ClickHouse Test Group Environment

 export REGULAR_INFRA_DATADIR="/var/lib/proxysql"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/tap/groups/legacy-clickhouse/env.sh` around lines 1 - 3, Add a shebang
as the first line of the env file to identify the shell dialect (e.g.,
#!/usr/bin/env bash) so static analysis tools recognize it; update the top of
the file containing the REGULAR_INFRA_DATADIR export to begin with the shebang
line (before any comments or exports) to avoid changing behavior when the file
is sourced.

test/infra/control/start-proxysql-isolated.bash (1)

87-89: ⚠️ Potential issue | 🟡 Minor

Avoid piping the final exec.

exec on the left side of a pipeline only replaces the pipeline child, so ProxySQL still is not PID 1 here, and its exit status is still hidden behind tee.

Make ProxySQL the direct exec target

-exec /usr/bin/proxysql --idle-threads --clickhouse-server --sqlite3-server -f -c /etc/proxysql.cnf -D /var/lib/proxysql 2>&1 | tee /var/lib/proxysql/proxysql.log
+exec /usr/bin/proxysql --idle-threads --clickhouse-server --sqlite3-server -f -c /etc/proxysql.cnf -D /var/lib/proxysql > >(tee /var/lib/proxysql/proxysql.log) 2>&1

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 87 - 89, The
current line uses exec on the left side of a pipeline so ProxySQL is not PID 1
and its exit status is obscured; change the invocation so /usr/bin/proxysql is
exec'd directly and its stdout/stderr are tee'd via process substitution (e.g.
use exec /usr/bin/proxysql ... -D /var/lib/proxysql > >(tee
/var/lib/proxysql/proxysql.log) 2>&1) so ProxySQL becomes PID 1 and logs still
go to /var/lib/proxysql/proxysql.log; also remove the stray trailing
double-quote from the command.

test/infra/control/start-proxysql-isolated.bash (3)

56-58: ⚠️ Potential issue | 🟠 Major

The global unset disables coverage for the primary ProxySQL process.

The unset GCOV_PREFIX GCOV_PREFIX_STRIP at lines 57-58 runs before the primary ProxySQL starts at line 99, so the primary instance won't write coverage data to /gcov. The intent was to disable coverage only for background cluster nodes.

Fix: use `env -u` only for background node launches

Remove lines 57-58 and modify line 94 to unset the variables only for background processes:

-# Disable gcov for background cluster nodes to avoid concurrent .gcda writes
-unset GCOV_PREFIX GCOV_PREFIX_STRIP
-
 # Start cluster nodes as background processes
 for i in \$(seq 1 ${NUM_NODES}); do
     ...
-    /usr/bin/proxysql --idle-threads -f -c \${NODE_DIR}/proxysql-node.cnf -D \${NODE_DIR} >> \${NODE_DIR}/proxysql.log 2>&1 &
+    env -u GCOV_PREFIX -u GCOV_PREFIX_STRIP /usr/bin/proxysql --idle-threads -f -c \${NODE_DIR}/proxysql-node.cnf -D \${NODE_DIR} >> \${NODE_DIR}/proxysql.log 2>&1 &

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 56 - 58, The
global unset of GCOV_PREFIX and GCOV_PREFIX_STRIP inside the STARTUP_CMD removes
coverage for the primary ProxySQL process; remove the lines that unset these
variables from STARTUP_CMD and instead apply "env -u" when spawning background
cluster nodes (the commands that launch background instances), so only
background node launches have GCOV_PREFIX/GCOV_PREFIX_STRIP removed while the
primary process retains coverage.

---

24-28: ⚠️ Potential issue | 🟠 Major

Add input validation for NUM_NODES to prevent port collisions.

If PROXYSQL_CLUSTER_NODES is set to 10 or higher, the calculated ports will collide with primary pgsql ports (e.g., node 10: admin=6132 collides with pgsql admin). Non-integer values would also cause arithmetic errors.

Suggested validation

 # Cluster configuration
 NUM_NODES=${PROXYSQL_CLUSTER_NODES:-9}
 if [[ "${SKIP_CLUSTER_START}" == "1" ]] || [[ "${SKIP_CLUSTER_START}" == "true" ]]; then
     NUM_NODES=0
 fi
+if ! [[ "${NUM_NODES}" =~ ^[0-9]$ ]]; then
+    echo "Error: PROXYSQL_CLUSTER_NODES must be an integer between 0 and 9."
+    exit 1
+fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 24 - 28, The
NUM_NODES assignment using PROXYSQL_CLUSTER_NODES lacks validation and can
accept non-integers or values >=10 causing port collisions; add input validation
after NUM_NODES is set (and after honoring SKIP_CLUSTER_START) to ensure
NUM_NODES is an integer between 0 and 9 inclusive, and if invalid print a clear
error to stderr and exit with non-zero status. Validate the value referenced by
PROXYSQL_CLUSTER_NODES (and the resulting NUM_NODES) using a numeric check and
range check, and handle non-integer input gracefully (error+exit) rather than
letting bash arithmetic or port calculation proceed; update any error messages
to reference NUM_NODES/PROXYSQL_CLUSTER_NODES and SKIP_CLUSTER_START for
clarity.

---

98-100: ⚠️ Potential issue | 🟡 Minor

The exec ... | tee pattern prevents proper signal propagation to the ProxySQL process.

When exec is combined with a pipe, the shell creates a subshell for the pipeline, so exec replaces only the subshell rather than the main shell process. Signals sent to PID 1 may not reach ProxySQL directly.

Alternative: use process substitution to preserve exec semantics

-exec /usr/bin/proxysql --idle-threads --clickhouse-server --sqlite3-server -f -c /etc/proxysql.cnf -D /var/lib/proxysql 2>&1 | tee /var/lib/proxysql/proxysql.log
+exec /usr/bin/proxysql --idle-threads --clickhouse-server --sqlite3-server -f -c /etc/proxysql.cnf -D /var/lib/proxysql > >(tee /var/lib/proxysql/proxysql.log) 2>&1

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 98 - 100, The
current line uses "exec /usr/bin/proxysql ... | tee
/var/lib/proxysql/proxysql.log", which breaks signal propagation because exec is
used with a pipe; change it to preserve exec semantics by redirecting ProxySQL's
stdout/stderr into a process substitution instead of a pipeline (so PID 1
becomes the proxysql binary). Update the invocation that references
/usr/bin/proxysql and /var/lib/proxysql/proxysql.log to use process substitution
(or an equivalent direct redirection) so signals delivered to PID 1 are received
by the ProxySQL process.

test/infra/control/start-proxysql-isolated.bash (3)

126-139: Switch [ to [[ for SonarCloud compliance.

SonarCloud flags the use of single brackets for conditional tests. Using [[ is safer and more feature-rich in Bash.

Suggested fix

-while [ $COUNT -lt $MAX_WAIT ]; do
+while [[ $COUNT -lt $MAX_WAIT ]]; do
     ...
 done
-if [ $COUNT -ge $MAX_WAIT ]; then echo " TIMEOUT"; exit 1; fi
+if [[ $COUNT -ge $MAX_WAIT ]]; then echo " TIMEOUT"; exit 1; fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 126 - 139,
Replace POSIX-style test brackets with Bash extended tests: change the while and
final if conditions that use [ $COUNT -lt $MAX_WAIT ] and [ $COUNT -ge $MAX_WAIT
] to use [[ $COUNT -lt $MAX_WAIT ]] and [[ $COUNT -ge $MAX_WAIT ]], and also
ensure the inner if that checks the docker exec command uses [[ ... ]] form for
its conditional (keeping the same command invocation with PROXY_CONTAINER,
COUNT, MAX_WAIT variables and the docker exec mysql calls unchanged).

---

159-166: Switch [ to [[ in cluster initialization conditionals for SonarCloud compliance.

Lines 159 and 166 use single brackets which SonarCloud flags.

Suggested fix

-if [ "${NUM_NODES}" -gt 0 ]; then
+if [[ "${NUM_NODES}" -gt 0 ]]; then
     echo ">>> Initializing ProxySQL Cluster (${NUM_NODES} nodes)..."
     ...
     CORE_NODES=3
-    if [ "${NUM_NODES}" -lt 3 ]; then CORE_NODES="${NUM_NODES}"; fi
+    if [[ "${NUM_NODES}" -lt 3 ]]; then CORE_NODES="${NUM_NODES}"; fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 159 - 166,
Replace POSIX test brackets with Bash conditional syntax to satisfy SonarCloud:
update the conditionals that use NUM_NODES (the checks controlling cluster
initialization and CORE_NODES assignment) from if [ "${NUM_NODES}" -gt 0 ]; then
and if [ "${NUM_NODES}" -lt 3 ]; then to use [[ ... ]] (e.g., if [[
"${NUM_NODES}" -gt 0 ]]; then and if [[ "${NUM_NODES}" -lt 3 ]]; then), leaving
the surrounding logic (MYSQL_CMD, CORE_NODES assignment) unchanged.

---

141-156: Switch [ to [[ in cluster node wait loop for SonarCloud compliance.

Lines 146 and 155 use single brackets which SonarCloud flags.

Suggested fix

-    while [ $COUNT -lt $MAX_WAIT ]; do
+    while [[ $COUNT -lt $MAX_WAIT ]]; do
         ...
     done
-    if [ $COUNT -ge $MAX_WAIT ]; then echo " TIMEOUT (node ${i})"; exit 1; fi
+    if [[ $COUNT -ge $MAX_WAIT ]]; then echo " TIMEOUT (node ${i})"; exit 1; fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/infra/control/start-proxysql-isolated.bash` around lines 141 - 156,
Replace the POSIX test brackets with Bash conditional brackets in the cluster
wait loop: change the while and if tests that currently use [ $COUNT -lt
$MAX_WAIT ] and [ $COUNT -ge $MAX_WAIT ] to use [[ ... ]] (e.g., [[ $COUNT -lt
$MAX_WAIT ]] and [[ $COUNT -ge $MAX_WAIT ]]) for SonarCloud compliance and safer
evaluation of variables like COUNT and MAX_WAIT in the loop that references
NUM_NODES, ADMIN_PORT, PROXY_CONTAINER, COUNT and MAX_WAIT.